Support autocorrection even if `reject` is used on `Performance/Count` #307

r7kamura · 2022-10-01T03:29:44Z

Until now, autocorrect was given up when reject was used, but it is now supported.

# bad
array.reject { |e| e > 2 }.size

# good
array.count { |e| !(e > 2) }

# bad
array.reject { |e| e > 2 }.count

# good
array.count { |e| !(e > 2) }

# bad
array.reject { |e| e > 2 }.length

# good
array.count { |e| !(e > 2) }

# bad
array.reject(&:value).size

# good
array.count { |element| !element.value }

# bad
array.reject(&block).size

# good
array.count { !block.call }

# bad
array.reject {
  foo
  bar
}.length

# good
array.count {
  foo
  !(bar)
}

The last one looks a bit weird but I think other cops autocorrects it to some other good form. If there is a better way to negate statements, I'd like to use it.

Before submitting the PR make sure the following are checked:

The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
Wrote good commit messages.
Commit message starts with [Fix #issue-number] (if the related issue exists).
Feature branch is up-to-date with master (if not - rebase it).
Squashed related commits together.
Added tests.
Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

koic · 2022-10-05T05:26:39Z

spec/rubocop/cop/performance/count_spec.rb

+        array.count {
+          !(foo
+          bar)
+        }


It looks like it needs to be autocorrected to the following for compatible.

Suggested change

array.count {

!(foo

bar)

}

array.count {

foo

!bar

}

I think that is the point I mentioned at the end of the description, do you have any better ideas to achieve that, and what about this thought?

other cops autocorrects it to some other good form

By the way, you say "for compatible", what exactly is incompatible?

Ah..., it doesn't contribute to compatibility. But ! to foo makes no sense.

Perhaps the implementation that Solargraph uses to estimate the method's return type might be helpful:

https://github.com/castwide/solargraph/blob/12fcf71755db2d570f591de773753eb73ac0680c/lib/solargraph/parser/legacy/node_methods.rb#L226-L256

If we have a similar implementation, we can see which nodes can be the return value of a block, and if we negate them, I think we can achieve most of what we want to do. Though this may be a little too heavy implementation for this cop to handle.

I think it can be achieved by inverting the last sentence of block with !. Anyway, let's make a better autocorrect separately.

I changed it so that the only last statement is negated in the case of begin node as a simple enough approach. I think indeed it's going to work there, even if it's not a perfect autocorrection.

koic · 2022-10-09T17:26:54Z

Thanks!

koic reviewed Oct 5, 2022

View reviewed changes

Support autocorrection even if reject is used on Performance/Count

3554550

r7kamura force-pushed the feature/count branch from 65e43ed to 3554550 Compare October 5, 2022 21:33

koic merged commit 3dd06b5 into rubocop:master Oct 9, 2022

r7kamura deleted the feature/count branch October 9, 2022 20:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support autocorrection even if `reject` is used on `Performance/Count` #307

Support autocorrection even if `reject` is used on `Performance/Count` #307

r7kamura commented Oct 1, 2022 •

edited

Loading

koic Oct 5, 2022 •

edited

Loading

r7kamura Oct 5, 2022

koic Oct 5, 2022

r7kamura Oct 5, 2022 •

edited

Loading

koic Oct 5, 2022

r7kamura Oct 5, 2022

koic commented Oct 9, 2022

Support autocorrection even if reject is used on Performance/Count #307

Support autocorrection even if reject is used on Performance/Count #307

Conversation

r7kamura commented Oct 1, 2022 • edited Loading

koic Oct 5, 2022 • edited Loading

Choose a reason for hiding this comment

r7kamura Oct 5, 2022

Choose a reason for hiding this comment

koic Oct 5, 2022

Choose a reason for hiding this comment

r7kamura Oct 5, 2022 • edited Loading

Choose a reason for hiding this comment

koic Oct 5, 2022

Choose a reason for hiding this comment

r7kamura Oct 5, 2022

Choose a reason for hiding this comment

koic commented Oct 9, 2022

Support autocorrection even if `reject` is used on `Performance/Count` #307

Support autocorrection even if `reject` is used on `Performance/Count` #307

r7kamura commented Oct 1, 2022 •

edited

Loading

koic Oct 5, 2022 •

edited

Loading

r7kamura Oct 5, 2022 •

edited

Loading